docs: clarify Service.interface(), VarlinkError, RequestHandler, and server classes by haraldh · Pull Request #87 · varlink/python

haraldh · 2026-02-12T11:11:41Z

Improve pydoc documentation to address common points of confusion:

Service.interface() expects a varlink interface name that maps to a .varlink file in interface_dir, not a Python class
VarlinkError subclasses for user-defined errors require manually constructing the {"error": ..., "parameters": {...}} dict
RequestHandler requires a service class variable wired to the Service instance
ThreadingServer and ForkingServer live in the varlink package; add docstrings and list all server classes in the module docstring

Copilot

Pull request overview

Documentation-focused PR to reduce common user confusion around implementing varlink services/servers and raising custom VarlinkErrors.

Changes:

Clarifies Service.interface() usage and how .varlink files are located/loaded.
Adds guidance on wiring Service into a RequestHandler subclass and running servers.
Expands docs for VarlinkError subclasses and adds docstrings for ThreadingServer/ForkingServer; updates package-level server guidance.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
`varlink/server.py`	Enhances pydoc for `Service.interface()`, `RequestHandler`, and server classes; adds examples and server-class docstrings.
`varlink/error.py`	Expands `VarlinkError` docstring with guidance for user-defined errors.
`varlink/__init__.py`	Updates module docstring to explain recommended server setup and enumerates server classes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T11:20:02Z

varlink/server.py

-    specified in the constructor of the Service. It has to end in '.varlink'.
+    The methods defined on the decorated class directly implement the varlink
+    interface methods. Each method receives the varlink call parameters as
+    keyword arguments and must return a dict matching the method's return type.


The docstring says handler methods receive varlink call parameters as keyword arguments, but Service._handle currently invokes methods with positional arguments (in the order of method.in_type.fields) and only uses keyword args for the special _raw/_message/_more/etc parameters. This wording is misleading; update it to describe positional args (and optionally mention the special keyword-only parameters).

Suggested change

keyword arguments and must return a dict matching the method's return type.

positional arguments (in the order of the method's input fields) and must

return a dict matching the method's return type. Additional information may

be passed via special keyword-only parameters (such as ``_raw``, ``_message``,

or ``_more``), if supported by the Service implementation.

Copilot · 2026-02-12T11:20:02Z

varlink/server.py

+        >>> class ServiceRequestHandler(varlink.RequestHandler):
+        >>>     service = service
+        >>>
+        >>> server = varlink.ThreadingServer("unix:@example", ServiceRequestHandler)


The example mixes unqualified Service(...) with varlink.RequestHandler/varlink.ThreadingServer, but it never shows import varlink (or from varlink import Service). In a typical import varlink usage, Service won’t be in scope, so the snippet is not directly runnable as written. Consider either fully-qualifying the Service construction (varlink.Service(...)) or dropping the varlink. prefixes and keeping everything module-local.

Suggested change

>>> class ServiceRequestHandler(varlink.RequestHandler):

>>> service = service

>>>

>>> server = varlink.ThreadingServer("unix:@example", ServiceRequestHandler)

>>> class ServiceRequestHandler(RequestHandler):

>>> service = service

>>>

>>> server = ThreadingServer("unix:@example", ServiceRequestHandler)

Copilot · 2026-02-12T11:20:03Z

varlink/server.py

+        :param filename: A varlink interface name (e.g. ``'com.example.service'``).
+            The file ``{filename}.varlink`` is loaded from the ``interface_dir`` given
+            to the Service constructor. This must be an interface name or a file path,
+            **not** a Python class or type.


Service.interface() docstring says filename can be an “interface name or a file path”, but the implementation treats non-absolute values as interface names: it always appends .varlink and prefixes interface_dir. Only absolute paths are opened as-is. Clarify this behavior (and whether callers should pass an absolute path to a .varlink file when using a path).

Suggested change

:param filename: A varlink interface name (e.g. ``'com.example.service'``).

The file ``{filename}.varlink`` is loaded from the ``interface_dir`` given

to the Service constructor. This must be an interface name or a file path,

**not** a Python class or type.

:param filename: Either (1) a varlink interface name (e.g.

``'com.example.service'``), in which case the file

``{filename}.varlink`` is loaded from the ``interface_dir`` given to the

Service constructor, or (2) an absolute path to a ``.varlink`` file,

which is opened directly. Any non-absolute value is treated as an

interface name (with ``.varlink`` appended and ``interface_dir``

prepended), so callers who want to load by path must pass an absolute

path. This must be an interface name or a file path, **not** a Python

class or type.

Copilot · 2026-02-12T11:20:03Z

varlink/__init__.py

+``@service.interface()`` decorator, wire it to a :class:`varlink.RequestHandler` subclass, and run it
+with one of the server classes:
+
+- :class:`varlink.Server` -- single-connection base server (analogous to ``socketserver.TCPServer``)


The module docstring describes varlink.Server as a “single-connection base server”, but Server (like socketserver.TCPServer) can accept multiple connections sequentially; it’s single-threaded / non-concurrent rather than single-connection. Rewording would avoid implying it only handles one connection total.

Suggested change

- :class:`varlink.Server` -- single-connection base server (analogous to ``socketserver.TCPServer``)

- :class:`varlink.Server` -- single-threaded base server for sequential connections (analogous to ``socketserver.TCPServer``)

phlogistonjohn · 2026-02-12T13:12:50Z

varlink/server.py

+        Import from the ``varlink`` package (``varlink.ForkingServer``), **not** from
+        ``socketserver``.


FWIW I find this and similar sentences in the patch unclear. I think it's trying to say that other components of varlink library expect this class and not the somewhat similarly named class in socketserver correct? I think it could be worded better in that case, especially since there's no ForkingServer in socketserver (the closest are ForkingTCPServer and other ForkingServer classes - AFAICT).

Maybe it's me, but this part doesn't look changed. I also don't feel strongly about it and was just providing feedback.

good catch! Thanks

behrmann

Nice docs improvement, just two minor comments.

behrmann · 2026-02-12T21:15:10Z

varlink/error.py


 class VarlinkError(Exception):
-    """The base class for varlink error exceptions"""
+    """The base class for varlink error exceptions.


A small nit: I think in the Python world it's clear that errors are modelled as exceptions, so this is a bit doubled

Suggested change

"""The base class for varlink error exceptions.

"""The base class for varlink errors.

behrmann · 2026-02-12T21:18:56Z

varlink/error.py

+        # Given:  error ActionFailed (field: string)  in the .varlink file
+
+        class ActionFailed(VarlinkError):
+            def __init__(self, reason):
+                VarlinkError.__init__(self, {
+                    "error": "com.example.ActionFailed",
+                    "parameters": {"field": reason},
+                })


I think we can spell this out more clearly:

Suggested change

# Given: error ActionFailed (field: string) in the .varlink file

class ActionFailed(VarlinkError):

def __init__(self, reason):

VarlinkError.__init__(self, {

"error": "com.example.ActionFailed",

"parameters": {"field": reason},

})

# Given: error ActionFailed (field: string) in an org.example.myinterface.varlink file

class ActionFailed(VarlinkError):

def __init__(self, reason):

VarlinkError.__init__(self, {

"error": "org.example.myinterface.ActionFailed",

"parameters": {"field": reason},

})

Also changing com.example to org.example, because it's the more common combination, I think.

behrmann · 2026-02-12T21:20:27Z

varlink/server.py

    def interface(self, filename):
+        """Decorator that registers a class as the handler for a varlink interface.
+
+        :param filename: A varlink interface name (e.g. ``'com.example.service'``).


I'd go here and below with org as well.

…server classes Improve pydoc documentation to address common points of confusion: - Service.interface() expects a varlink interface name that maps to a .varlink file in interface_dir, not a Python class - VarlinkError subclasses for user-defined errors require manually constructing the {"error": ..., "parameters": {...}} dict - RequestHandler requires a `service` class variable wired to the Service instance - ThreadingServer and ForkingServer live in the varlink package, not socketserver; add docstrings and list all server classes in the module docstring

haraldh temporarily deployed to pypi February 12, 2026 11:15 — with GitHub Actions Inactive

haraldh requested review from behrmann and Copilot February 12, 2026 11:16

Copilot started reviewing on behalf of haraldh February 12, 2026 11:16 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

phlogistonjohn reviewed Feb 12, 2026

View reviewed changes

behrmann approved these changes Feb 12, 2026

View reviewed changes

haraldh force-pushed the hh/doc-improvement branch from 67483da to 263f0bd Compare February 13, 2026 10:18

haraldh had a problem deploying to pypi February 13, 2026 10:18 — with GitHub Actions Failure

haraldh had a problem deploying to pypi February 13, 2026 10:19 — with GitHub Actions Failure

haraldh requested review from behrmann and phlogistonjohn February 13, 2026 10:21

behrmann approved these changes Feb 13, 2026

View reviewed changes

haraldh force-pushed the hh/doc-improvement branch from 263f0bd to 37d0a02 Compare February 16, 2026 08:00

haraldh deployed to pypi February 16, 2026 08:00 — with GitHub Actions View deployment

haraldh force-pushed the hh/doc-improvement branch from 37d0a02 to a64ff80 Compare February 16, 2026 08:02

haraldh had a problem deploying to pypi February 16, 2026 08:02 — with GitHub Actions Failure

haraldh merged commit 3292a46 into master Feb 16, 2026
8 of 9 checks passed

haraldh deleted the hh/doc-improvement branch February 16, 2026 08:04

-    keyword arguments and must return a dict matching the method's return type.
+    positional arguments (in the order of the method's input fields) and must
+    return a dict matching the method's return type. Additional information may
+    be passed via special keyword-only parameters (such as ``_raw``, ``_message``,
+    or ``_more``), if supported by the Service implementation.

-        :param filename: A varlink interface name (e.g. ``'com.example.service'``).
-            The file ``{filename}.varlink`` is loaded from the ``interface_dir`` given
-            to the Service constructor. This must be an interface name or a file path,
-            **not** a Python class or type.
+        :param filename: Either (1) a varlink interface name (e.g.
+            ``'com.example.service'``), in which case the file
+            ``{filename}.varlink`` is loaded from the ``interface_dir`` given to the
+            Service constructor, or (2) an absolute path to a ``.varlink`` file,
+            which is opened directly. Any non-absolute value is treated as an
+            interface name (with ``.varlink`` appended and ``interface_dir``
+            prepended), so callers who want to load by path must pass an absolute
+            path. This must be an interface name or a file path, **not** a Python
+            class or type.

	- :class:`varlink.Server` -- single-connection base server (analogous to ``socketserver.TCPServer``)
	- :class:`varlink.Server` -- single-threaded base server for sequential connections (analogous to ``socketserver.TCPServer``)

		Import from the ``varlink`` package (``varlink.ForkingServer``), not from
		``socketserver``.

	"""The base class for varlink error exceptions.
	"""The base class for varlink errors.

Conversation

haraldh commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

behrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haraldh commented Feb 12, 2026 •

edited

Loading